refactor(workspace): dissolve ironclaw_storage + HsmBackend placeholder - #3678
Closed
ilblackdragon wants to merge 2 commits into
Closed
ilblackdragon wants to merge 2 commits into
ilblackdragon wants to merge 2 commits into
Conversation
The ironclaw_storage crate predates the unified RootFilesystem surface introduced by PR #3659 (universal FS dispatch). Its `BlobStore`/`RecordStore` traits, `StorageKey`/`StorageVersion`/`PutCondition` types, and `StoredBlob`/`StoredRecord` shapes parallel the new unified put/get /CasExpectation/RecordVersion machinery on `RootFilesystem` — a textbook duplicate-dispatch smell flagged by .claude/rules/architecture.md. Only `ironclaw_outbound` consumed any of the crate, and only 5 small helpers (`encode_json`, `decode_json`, `redacted_backend_error`, `StorageError::Backend`, `ABSENT_SCOPE_COMPONENT`). All other types and the entire `BlobStore`/`RecordStore` surface (660 LOC) were unused — their intended consumers already moved to `RootFilesystem` directly. Inlined the 5 helpers into `crates/ironclaw_outbound/src/db.rs`: - `encode_json`/`decode_json` → direct `serde_json::to_string`/`from_str` - `redacted_backend_error` → local log+collapse to `OutboundError::Backend` (preserves the redaction boundary required by ironclaw_outbound/CLAUDE.md) - `ABSENT_SCOPE_COMPONENT` → local const "" Removed the crate's workspace membership, the outbound dep, the forbidden-edges BoundaryRule, and the crate directory. Also updated the ironclaw_outbound BoundaryRule to permit a normal dependency on `ironclaw_filesystem` — `FilesystemOutboundStateStore` landed in the prior cascade PR and the boundary rule was stale.
…egacy Two changes that close out the demoable parts of the universal-FS-dispatch rework (tasks #18 and the demonstrable portion of #19 from the plan). **HsmBackend placeholder** (`crates/ironclaw_filesystem/src/hsm.rs`). Demonstrates that a new backend is a single-file change: implements the one `RootFilesystem` trait, declares a restricted capability surface (`Read` + `Write` + `Stat` + `Delete` + `TxnCapability::Cas` — no records, no query, no index, no events, no multi-key transactions), and routes `put`/`get`/`delete`/`stat`/`list_dir` through an in-process placeholder. Five tests prove the seam works end-to-end: - `hsm_supports_encrypted_bytes_round_trip` — bytes put/get works. - `hsm_rejects_structured_records` — `put` with `RecordKind::Some` or non-empty `indexed` returns `Unsupported`, so a consumer cannot accidentally route records through encryption-only storage. - `hsm_rejects_query_and_index_ops` — `query`/`ensure_index` return `Unsupported` consistent with the declared capabilities. - `composite_rejects_overclaimed_hsm_descriptor` — mount-time validation (`validate_mount_capabilities`) refuses a descriptor that claims `Query`/`IndexExact` over a backend that doesn't deliver, failing with `FilesystemError::DescriptorOverclaims { missing, .. }`. - `composite_routes_to_hsm_under_secrets_mount` — the acceptance gate: mounting HsmBackend at `/secrets` and routing put/get through the composite works with no consumer-visible changes. Indexed projection is still rejected because the declared capabilities advertise no index/query support. A real HSM implementation replaces the in-memory placeholder with an HSM session handle; the trait surface, capability declarations, and mount-time validation are reusable as-is. The placeholder is *not* a security boundary — it is a seam demonstration. **database.md scoped to legacy directories**. The dual-backend rule file (`.claude/rules/database.md`) is `paths`-scoped to `src/db/**`, `src/history/**`, and `migrations/**` — exactly the legacy surface that predates the universal FS dispatch. Added a "Status & Direction" preamble pointing new persistence work at `ScopedFilesystem` and the `2026-05-14-universal-fs-dispatch.md` plan, with the existing per-crate dual-backend guidance kept (and tagged "legacy") for code still inside those directories.
github-actions
Bot
added
scope: docs
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request migrates the persistence architecture from per-crate storage traits to a universal RootFilesystem mount table system. As part of this transition, the ironclaw_storage crate has been removed, and its core utilities have been inlined into the ironclaw_outbound crate. A new HsmBackend was added to ironclaw_filesystem to demonstrate the new dispatch model. Feedback indicates that HsmBackend should explicitly declare Capability::List in its capabilities to remain consistent with its implementation of the list_dir method.
Comment on lines
+53
to
+60
| fn declared_capabilities() -> BackendCapabilities { | ||
| BackendCapabilities::empty() | ||
| .with(Capability::Read) | ||
| .with(Capability::Write) | ||
| .with(Capability::Stat) | ||
| .with(Capability::Delete) | ||
| .with_txn(TxnCapability::Cas) | ||
| } |
Contributor
There was a problem hiding this comment.
The HsmBackend implements list_dir by delegating to the inner backend, but it does not declare Capability::List in its capabilities. While the legacy capability plane is currently descriptor-driven for validation, new backends should accurately advertise their supported operations for consistency and future-proofing.
fn declared_capabilities() -> BackendCapabilities {
BackendCapabilities::empty()
.with(Capability::Read)
.with(Capability::Write)
.with(Capability::List)
.with(Capability::Stat)
.with(Capability::Delete)
.with_txn(TxnCapability::Cas)
}References
- Capabilities should be handled consistently across all capability kinds and accurately advertised on all surfaces.
5 of 7 tasks
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two follow-on commits on top of the universal-FS-dispatch cascade. Closes tasks #9, #18, and the demonstrable portion of #19 of the rework plan.
Summary
Commit 1: dissolve
ironclaw_storagecrate.The crate predated the unified
RootFilesystemsurface introduced by PR #3659. ItsBlobStore/RecordStoretraits,StorageKey/StorageVersion/PutConditiontypes, andStoredBlob/StoredRecordshapes parallel the new unifiedput/get/CasExpectation/RecordVersionmachinery — a textbook duplicate-dispatch smell from.claude/rules/architecture.md.Only
ironclaw_outboundconsumed any of the crate, and only 5 small helpers (encode_json,decode_json,redacted_backend_error,StorageError::Backend,ABSENT_SCOPE_COMPONENT). All other types and the entireBlobStore/RecordStoresurface (660 LOC) were unused — their intended consumers already moved toRootFilesystemdirectly.crates/ironclaw_outbound/src/db.rsironclaw_storage = { path = ... }from outboundCargo.toml\"crates/ironclaw_storage\"from workspace membersironclaw_storagefrom architecture testsironclaw_outboundBoundaryRule to permitironclaw_filesystem(it was stale:FilesystemOutboundStateStorelanded in the prior cascade PR feat(outbound): FilesystemOutboundStateStore on the unified surface #3670)Commit 2:
HsmBackendplaceholder + database.md scoping.HsmBackend(crates/ironclaw_filesystem/src/hsm.rs) demonstrates the universal-dispatch seam end-to-end: one trait, one file, declared restricted capabilities, mount-time validation rejects over-claiming descriptors, swap-by-wiring works without consumer edits. Five tests cover all of those gates including the rejection paths. A production HSM swaps the in-process placeholder for a real session handle; the trait + capability machinery is reusable as-is..claude/rules/database.mdis now scoped via itspaths:frontmatter tosrc/db/**,src/history/**, andmigrations/**— exactly the legacy directories that predate the universal FS dispatch — with a 'Status & Direction' preamble pointing new persistence work atScopedFilesystemand the rework plan.Test plan
Stack
Built on top of #3672 (reborn/fs-run-state) → #3671 (authorization) → #3670 (outbound) → #3666 (processes) → #3659 (foundation).
What this does NOT do
Tasks #10, #14, #15, #16, #17 of the rework plan are still outstanding (secrets, event_store, memory, engine Store, src/db dissolution). Each is a substantial separate PR. The full grep-acceptance gate from #19 (≤1 store per crate, cfg-gates centralized in filesystem) cannot be met until those migrations land.